Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IDE doc for better auto-completion #774

Merged
merged 2 commits into from
Feb 14, 2015
Merged

Conversation

bakura10
Copy link
Contributor

On modern IDE (at least on PHPStorm), returning static allows to let the IDE resolve on real-time based on the caller type. This is necessary if we still want to have nice autocompletion with the fluent interface. I've only fixed that for param and aggregations, I'll do another PR in the future if I find other cases :)

@im-denisenko
Copy link
Contributor

I always thought that @return static or @return self should be used for static methods like Elastica\Search::create, but in case of fluent interface @return $this is more semantic, isn't it? This differences also described in proposed psr-5 (13, 14, 15).

// my ST3 cannot into autocomplete, so maybe i'm just wrong :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.47%) to 83.54% when pulling 59d6a52 on bakura10:fix-ide into 0db1e7e on ruflin:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.47%) to 83.54% when pulling 59d6a52 on bakura10:fix-ide into 0db1e7e on ruflin:master.

@bakura10
Copy link
Contributor Author

I just tried and I can confirm that $this work as well :). I like static because it is coherent with how late-state binding works, but I'm open to change that to $this. Let's let @ruflin decide :D.

@ruflin
Copy link
Owner

ruflin commented Feb 10, 2015

I wasn't even aware that PHPStorm is now able to use static or $this for auto complete. I quite like "$this" as it describes very well what it actually is. In which IDE's is this going to work?

@bakura10
Copy link
Contributor Author

I'm pretty sure NetBeans take advantage of that!

@im-denisenko
Copy link
Contributor

Eclipse confirmed also

@bakura10
Copy link
Contributor Author

I've updated to modify static to $this!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.93%) to 84.0% when pulling c301d14 on bakura10:fix-ide into 0db1e7e on ruflin:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.93%) to 84.0% when pulling c301d14 on bakura10:fix-ide into 0db1e7e on ruflin:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.93%) to 84.0% when pulling c301d14 on bakura10:fix-ide into 0db1e7e on ruflin:master.

ruflin added a commit that referenced this pull request Feb 14, 2015
Fix IDE doc for better auto-completion
@ruflin ruflin merged commit bbbf9d3 into ruflin:master Feb 14, 2015
@ruflin
Copy link
Owner

ruflin commented Feb 14, 2015

Merged. Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants